Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Charbeat config values for Feature Index page #7941

Merged
merged 9 commits into from Oct 5, 2020
Merged

Add Charbeat config values for Feature Index page #7941

merged 9 commits into from Oct 5, 2020

Conversation

chewu
Copy link
Contributor

@chewu chewu commented Sep 28, 2020 •
edited

Resolves #7910

Overall change:
We are using the same values as the Index page as instructed by @JonBeeb.

Code changes:

  • A bullet point list of key code changes that have been made.
  • When describing code changes, try to communicate how and why you implemented something a specific way, not just what has changed.
  • I have assigned myself to this PR and the corresponding issues
  • I have added the cross-team label to this PR if it requires visibility across World Service teams
  • I have assigned this PR to the Simorgh project

Testing:

  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
  • If necessary, I have run the local E2E non-smoke tests relevant to my changes (CYPRESS_APP_ENV=local CYPRESS_SMOKE=false npm run test:e2e:interactive)
  • This PR requires manual testing

@chewu chewu self-assigned this Sep 28, 2020
@chewu chewu added the ws-articles Tasks for the WS Articles Team label Sep 28, 2020
@chewu chewu added this to PR in Progress in Simorgh via automation Sep 28, 2020
@chewu chewu changed the title Add Charbeat values for Feature Index page Add Charbeat config values for Feature Index page Sep 28, 2020
karinathomasbbc
karinathomasbbc approved these changes Sep 29, 2020
View reviewed changes
Copy link
Collaborator

@karinathomasbbc karinathomasbbc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@simonsinclair simonsinclair moved this from PR in Progress to Code Review in Simorgh Sep 29, 2020
karinathomasbbc
karinathomasbbc approved these changes Sep 29, 2020
View reviewed changes
src/app/containers/ChartbeatAnalytics/utils/index.test.js Outdated
},
path: '/',
sections: 'Afrique, Afrique - FIX',
title: 'This is a Feautre Index page title',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? This is a Feature Index page title

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOH

@chris-hinds
Copy link
Contributor

Changes look good to me. Looks like you need to update the tests to reflect the Fix title change.

simonsinclair
simonsinclair approved these changes Oct 5, 2020
View reviewed changes
Copy link
Contributor

@simonsinclair simonsinclair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, Che. 👍

@karinathomasbbc karinathomasbbc moved this from Code Review to Ready for merge (probably) in Simorgh Oct 5, 2020
@chewu chewu merged commit 29ed23b into latest Oct 5, 2020
Simorgh automation moved this from Ready for merge (probably) to Done Oct 5, 2020
@chewu chewu deleted the 7910-chartbeat-values-feature-index-pages branch October 5, 2020 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonsinclair simonsinclair simonsinclair approved these changes

@chris-hinds chris-hinds chris-hinds approved these changes

@karinathomasbbc karinathomasbbc karinathomasbbc approved these changes

Assignees

@chewu chewu

Labels
ws-articles Tasks for the WS Articles Team
Projects
No open projects
Simorgh
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Define Chartbeat config values specific to the FIX page
4 participants
@chewu @chris-hinds @simonsinclair @karinathomasbbc